Skip to content

F-018: chore: add clippy.toml with disallowed-methods / disallowed-macros#21

Merged
Sephyi merged 4 commits intodevelopmentfrom
audit/f-018-clippy-toml
Apr 30, 2026
Merged

F-018: chore: add clippy.toml with disallowed-methods / disallowed-macros#21
Sephyi merged 4 commits intodevelopmentfrom
audit/f-018-clippy-toml

Conversation

@Sephyi
Copy link
Copy Markdown
Owner

@Sephyi Sephyi commented Apr 22, 2026

Summary

chore: add clippy.toml with disallowed-methods / disallowed-macros.

Audit context

Closes audit entry F-018 from #3.

Verification

  • cargo fmt --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test --all-targets

Note: one pre-existing test porcelain_exits_within_timeout_with_no_staged_changes is a known macOS cold-start flake that reproduces on unmodified development — unrelated to this change.

Add a minimal, pragmatic `clippy.toml` that catches two common
anti-patterns at lint time:

- `disallowed-methods` blocks `std::process::Command::new`, pushing
  async call-sites toward `tokio::process::Command` or
  `spawn_blocking`. Synchronous contexts (integration tests, CLI
  bootstrap before the runtime starts) opt out locally via
  `#[allow(clippy::disallowed_methods)]` with a comment.
- `disallowed-macros` blocks `std::dbg` so leftover debug scaffolding
  can't ship; `tracing::{debug,trace}` is the durable replacement.

Existing legitimate sync `Command` uses are annotated rather than
rewritten:

- `src/app.rs::hook_dir` runs in the synchronous handle_hook bootstrap
  path; F-002 will migrate it off sync Command, and the allow here
  documents the coordination so this PR doesn't block on F-002.
- `tests/history.rs` and `tests/porcelain.rs` set a module-level
  `#![allow(clippy::disallowed_methods)]` because integration tests
  legitimately shell out to git / the commitbee binary synchronously.

Closes audit entry F-018 from #3.
Copilot AI review requested due to automatic review settings April 22, 2026 19:51
@Sephyi Sephyi added the audit Codebase audit cleanup (issue #3) label Apr 22, 2026
@Sephyi Sephyi self-assigned this Apr 22, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a repository-wide clippy.toml configuration to enforce avoidance of blocking process spawning in async contexts and prevent shipping leftover debug macros, aligning with audit finding F-018.

Changes:

  • Introduces clippy.toml with disallowed-methods (blocking std::process::Command::new) and disallowed-macros (blocking dbg!).
  • Adds #[allow(clippy::disallowed_methods)] in select integration tests and in the hook path to avoid the new lint blocking existing code.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
clippy.toml Adds Clippy configuration for disallowed methods/macros tied to audit F-018.
src/app.rs Locally suppresses disallowed_methods for the git hook directory lookup.
tests/history.rs Suppresses disallowed_methods for git-fixture setup in history integration tests.
tests/porcelain.rs Suppresses disallowed_methods for porcelain contract tests that shell out to git/commitbee.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/app.rs Outdated
Comment thread tests/history.rs Outdated
Sephyi and others added 2 commits April 30, 2026 17:38
The justification comment is inaccurate: this code runs under the Tokio runtime (main is #[tokio::main] and Commands::Hook is handled inside app.run().await), so std::process::Command here can still block a runtime worker thread. Either update the comment to reflect the actual execution context and why blocking is acceptable temporarily, or (preferably) move the blocking call behind spawn_blocking / tokio::process::Command so the new lint remains meaningful in production code paths.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

Thank you for your contribution! Before we can merge this PR, you need to sign the Contributor License Agreement. To sign, please reply with a comment containing exactly: I have read the CLA Document and I hereby sign the CLA. You only need to sign once.


I have read the CLA Document and I hereby sign the CLA.


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Sephyi Sephyi merged commit 07659d7 into development Apr 30, 2026
7 of 11 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 30, 2026
@Sephyi Sephyi deleted the audit/f-018-clippy-toml branch April 30, 2026 17:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

audit Codebase audit cleanup (issue #3)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants